Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Get the integration tests up and running again #1300

Merged
merged 16 commits into from
Dec 27, 2024
Merged

Get the integration tests up and running again #1300

merged 16 commits into from
Dec 27, 2024

Conversation

BentiGorlich
Copy link
Member

  • The messenger transactional wrap is not working with the tests, as it conflicts with the DoctrineTestBundle
  • Fix some type problems (DateTime vs DateTimeImmutable, nulls and initialization errors)
  • SearchManager::findActivityPubActorsByUsername now converts !user@domain into @user@domain so searching for a magazine by !mag@domain works from the API as well
  • The StatsContentRepository had a sign error
  • add a new docker compose for the test dependencies, as well as oauth keys for the tests
  • remove the purge user test, as this is no longer an option
  • change the .env.test to fit the new dependencies

@BentiGorlich BentiGorlich added php Pull requests that update PHP code testing Unit/Functional/Integration test issues and pull requests labels Dec 17, 2024
@BentiGorlich BentiGorlich added this to the v1.7.4 milestone Dec 17, 2024
@BentiGorlich BentiGorlich self-assigned this Dec 17, 2024
@BentiGorlich BentiGorlich force-pushed the fix/tests branch 3 times, most recently from 82db8d1 to 18db1cc Compare December 17, 2024 15:33
@melroy89 melroy89 self-requested a review December 17, 2024 23:00
@BentiGorlich BentiGorlich force-pushed the fix/tests branch 4 times, most recently from e4f5cea to 9768db4 Compare December 18, 2024 08:36
@BentiGorlich
Copy link
Member Author

Ok, so it is now working, but the tests take forever: 23 minutes. There is a library called paratest that utilizes parallel testing, however the transaction isolation is not working at the moment (I have no idea why) so we have random assert failures with it (just tested it)
@melroy89 if you have any idea what could cause the doctrine test bundle to not work, please let me know as this already caused some headache for me 😅

@BentiGorlich
Copy link
Member Author

Ok, so just checked what actually happens. It does not commit anything to the DB, however cahing is screwing us a little bit and the static settings were a problem. I am working on it :D

@melroy89
Copy link
Member

If you need more php extensions installed. Let me know right.

@BentiGorlich
Get the integration tests up and running again
e7e6822 
- The messenger transactional wrap is not working with the tests, as it conflicts with the DoctrineTestBundle
- Fix some type problems (DateTime vs DateTimeImmutable, nulls and initialization errors)
- `SearchManager::findActivityPubActorsByUsername` now converts `!user@domain` into `@user@domain` so searching for a magazine by `!mag@domain` works from the API as well
- The `StatsContentRepository` had a sign error
- add a new docker compose for the test dependencies, as well as oauth keys for the tests
- remove the purge user test, as this is no longer an option
- change the .env.test to fit the new dependencies
@BentiGorlich
Use the new pipeline image version 1.3
47ef994
@BentiGorlich
Fix the host for redis and psql
4b928a5
@BentiGorlich
Remove redis password
2e43c30
@BentiGorlich
Fix Composer cache, add npm setup and build
ad726c3
@BentiGorlich
Move to Valkey, try to fix hash error
983604f
@BentiGorlich
Working towards parallel test execution via paratest
5a5043c
@BentiGorlich
Consolidate unit and functional tests and fix unit tests
319db5d 
- put the unit and functional tests in the same CI job
- Fix the mention manager tests
@melroy89
Copy link
Member

melroy89 commented Dec 25, 2024 •
edited
Loading

That is strange you need this kernel interface object while not using it?

melroy89
melroy89 previously approved these changes Dec 25, 2024
View reviewed changes
@BentiGorlich
Copy link
Member Author

it is being passed to the parent which needs it to know in which environment it is

@BentiGorlich
Copy link
Member Author

the unit test job is required, but I renamed/removed it 😅

@BentiGorlich
Fix unit tests again
5deb08c
@BentiGorlich
Fix Tests after domain changes
371ec7c 
The domain changes from #1280 caused some tests to fail due to wrong assertions, fix that
@melroy89
Copy link
Member

Please don't rename integration tests to automated tests. You did so well. Why would you rename this?

Unit tests and integration tests are two separate tests. I would like to see which fails from the PR perspective.

@BentiGorlich
Copy link
Member Author

Ok so your comment reads relatively offensive 😅
My intention was to not have to have two redundant build steps only differing in one command. Additionally I wanted to make the build shorter as the unit tests will have to have the postgres and valkey services as well since the db bootstrap is always executed now. I can change it back if ypu really hate it. But since it is in two steps of The same check/run its easy to spot either way. Additionally for me it is only relevant that tests failed, not which kind of test failed

@melroy89
Copy link
Member

Ok so your comment reads relatively offensive 😅 My intention was to not have to have two redundant build steps only differing in one command. Additionally I wanted to make the build shorter as the unit tests will have to have the postgres and valkey services as well since the db bootstrap is always executed now. I can change it back if ypu really hate it. But since it is in two steps of The same check/run its easy to spot either way. Additionally for me it is only relevant that tests failed, not which kind of test failed

Ok Continue. I actually blame github.

@BentiGorlich BentiGorlich merged commit cc7210f into main Dec 27, 2024
7 checks passed
@BentiGorlich BentiGorlich deleted the fix/tests branch December 27, 2024 08:21
This was referenced Dec 31, 2024
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@melroy89 melroy89 melroy89 approved these changes

Assignees

@BentiGorlich BentiGorlich

Labels
php Pull requests that update PHP code testing Unit/Functional/Integration test issues and pull requests
Projects
None yet
Milestone
v1.7.4
Development

Successfully merging this pull request may close these issues.
2 participants
@BentiGorlich @melroy89